[charts] Support animation #9926

alexfauquette · 2023-08-04T12:37:05Z

The current solution is to use react-spring transition.

To let Argos pass, I added a config that disabled the animation.

The demonstration: https://deploy-preview-9926--material-ui-x.netlify.app/x/react-charts/bars/#animation

It still suffers a small bug

In responsive mode, bars seem to come from the point (0, 0) instead of just growing from bottom to top.

It's because in responsive mode, the width of the chart is computed from its parent in a useEffect. So before the useEffect is done, the chart width is set by default to 0.

So from the point of view of useTransition the initial point is (0, 0) whereas it should be the correct one with height set to 0.

I think I will just block children's rendering at the first rendering in responsive. But if you have a better idea

github-actions · 2023-08-04T12:37:47Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

mui-bot · 2023-08-04T12:45:44Z

Deploy preview: https://deploy-preview-9926--material-ui-x.netlify.app/

Updated pages:

docs/data/charts/bars/bars.md

Generated by 🚫 dangerJS against 1ff3021

github-actions · 2023-08-07T06:55:58Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

flaviendelangle · 2023-08-08T12:39:45Z

packages/x-charts/src/BarChart/BarElement.tsx

@@ -117,12 +124,28 @@ export function BarElement(props: BarElementProps) {
  };
  const classes = useUtilityClasses(ownerState);

+  const spring = useSpring({


Do you know if our competitors are bundling a third party animation library like this?

Nivo is using spring

Rechart is using react-transition-group -> sounds limited for animating props modification

Echarts has its own set of function -> mAybe to much work for now

OK
One advantage of react-transition-group is that we already bundle it for the pickers so most users will have it in their bundle anyway.
But if it's too limited, I agree that going for react-spring can be interesting.

From What I understand, react-transition-group is mostly to create transition during the creation/deletion of components. But here we will also be interested in props modification. For example adding a bar means pushing all the other ones to a new position

FWIW in my last projects I've always been using this minimalistic animation function, I was very satisfied with the choice because it's very simple to understand & tune. Adding a wrapper around it to make it into a hook should be trivial.

I spend some time to think about it, but will go with react-spring for now, more for their animated elements than for the interpolation functions. It allows to pass style as usual without having react re-render

https://www.react-spring.dev/docs/concepts/animated-elements

flaviendelangle

This is a nice addition and with a very little amount of new code.

If we agree to use react-spring (cc @LukasTy), then for me the only thing missing is a prop to disable the animation (because that's clearly something not everybody wants.
And disable it on the sparkline.

romgrk · 2023-09-27T15:46:05Z

packages/x-charts/src/BarChart/BarElement.tsx

@@ -64,6 +65,7 @@ export const BarElementPath = styled('rect', {
 export type BarElementProps = Omit<BarElementOwnerState, 'isFaded' | 'isHighlighted'> &
  React.ComponentPropsWithoutRef<'path'> & {
    highlightScope?: Partial<HighlightScope>;
+    yOrigine: number;


s/yOrigine/yOrigin/

I might admit I do not understand this comment 🙈

My bad, that's the sed CLI command way to perform a substitution, replacing yOrigine with yOrigin. "Origine" is French, the English word doesn't take a final e.

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

github-actions · 2023-09-29T14:52:24Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

alexfauquette · 2023-10-17T12:30:57Z

packages/x-charts/src/BarChart/BarPlot.tsx

  const { series, stackingGroups } = seriesData;
  const { xAxis, yAxis, xAxisIds, yAxisIds } = axisData;
  const defaultXAxisId = xAxisIds[0];
  const defaultYAxisId = yAxisIds[0];

+  const data = stackingGroups.flatMap(({ ids: groupIds }, groupIndex) => {


That can seem to be a huge modification, but it's "just" moving the computation of rectangle position at the top such that those coordinates can be passed to the useTransition

noraleonte

Amazing addition 🎉 💙

docs/data/charts/bars/bars.md

alexfauquette · 2023-10-17T14:07:46Z

About the animation starting at (0, 0) point, I confirm it get fixed by modifying the responsive container as follows. The idea being that if we have no width/height it's useless to do the upcoming computation since we will need to do them back for the next render when we get access to the correct sizing of the component.

 <ResizableContainer ref={containerRef} ownerState={{ width: inWidth, height: inHeight }}>
+      {width && height ? (
        <ChartContainer {...other} width={width} height={height} ref={ref} />
+      ) : null}
    </ResizableContainer>

LukasTy

The solution looks very elegant. 👍
One concern worth taking note of is that this additional library is currently 1/3 of the whole charts package in size. 🙈

General question:

Have you considered having the animation toggle top level (on the container), because I imagine that eventually we'll animate everything, even the tooltip movement. 🤔
Consider having a default for disableAnimation or animate, which would respect the prefers-reduced-motion os setting.
Similar to how we handle reduceAnimations on pickers.
Providing animation customization is probably a separate issue and planned for the future?
Have you looked over the argos differences? Some of those seem concerning and the result new doc page actually has overflowing bars. 🤔

docs/data/charts/bars/bars.md

packages/x-charts/src/BarChart/BarPlot.tsx

test/regressions/index.test.js

packages/x-charts/src/BarChart/BarPlot.tsx

alexfauquette · 2023-10-18T12:24:58Z

One concern worth taking note of is that this additional library is currently 1/3 of the whole charts package in size. 🙈

Yes, but it allows us to move forward. Maybe removing it for a lighter solution when we get a large userbase.

Have you considered having the animation toggle top level (on the container), because I imagine that eventually we'll animate everything, even the tooltip movement. 🤔

Good idea, but I'm not sure it's worth it.

Most of our users might use a single component, so they don't care.
If you're using composition, I assume you want fine-grained management

Consider having a default for disableAnimation or animate, which would respect the prefers-reduced-motion os setting.
Similar to how we handle reduceAnimations on pickers.

@react-spring/web already offers useReducedMotion() which remove animation. It's best that user call it once in their app. I thaught it was documented. I will add a sentence about it.

Providing animation customization is probably a separate issue and planned for the future?

Not really. In the issue, someone said that they just use default animation. We could consider providing a way to customize react-spring configuration, to get faster/slower animation. But more advanced customization seems to be a path in a difficult problem for no reward.

Have you looked over the argos differences? Some of those seem concerning and the result new doc page actually has overflowing bars. 🤔

What do you mean by "overflowing"?

For this kind of diff, the SVG edges definition is quite vague about how they are computed. So I assume that modifying the CPU usage can lead to edge errors.

Co-authored-by: Nora <72460825+noraleonte@users.noreply.github.com> Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

docs/data/charts/bars/bars.md

LukasTy · 2023-10-18T14:35:49Z

The latest Argos changes seem strange... As if the explicit skipAnimation definition no longer works... 🤷

alexfauquette · 2023-10-18T14:51:03Z

As if the explicit skipAnimation definition no longer works... 🤷

Yes, Argos use global settings and useReducedMotion override it.

I modified the hook such that it removes animation for users with a11y issue but do nothing otherwise

alexfauquette · 2023-10-18T15:06:16Z

It works :)

LukasTy · 2023-10-18T15:34:41Z

What do you mean by "overflowing"?

I think that the overflow regression issue might be coming from the change of precision in the translate coordinates. 🤔

Current release:

This PR

LukasTy

Great work! 💯
The precision question can be addressed separately. 👍

[charts] Add small animation

2e31fc1

alexfauquette marked this pull request as draft August 4, 2023 12:37

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 4, 2023

alexfauquette added 2 commits August 4, 2023 14:57

Merge remote-tracking branch 'upstream/master' into animation

728df48

fix

bc20a1b

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 4, 2023

alexfauquette added the component: charts This is the name of the generic UI component, not the React module! label Aug 4, 2023

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Aug 7, 2023

flaviendelangle reviewed Aug 8, 2023

View reviewed changes

flaviendelangle reviewed Aug 11, 2023

View reviewed changes

alexfauquette added 3 commits September 27, 2023 12:19

Merge remote-tracking branch 'upstream/master' into animation

aea58e5

updat ebar element for horizonal animation

5ffcf34

move all data management in a hook

686cf65

romgrk reviewed Sep 27, 2023

View reviewed changes

alexfauquette added 8 commits September 28, 2023 12:21

continue

27dc0b9

fix TS

04b0f03

Add animation demonstartion

d3caf28

Needs a props to setup animaion

806db91

props renaming

4053bf2

fix argox

fe6db79

Merge remote-tracking branch 'upstream/master' into animation

e2c0cae

fix argos test (try to)

5c9b182

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 28, 2023

try again

92cfdfd

alexfauquette marked this pull request as ready for review September 29, 2023 08:42

alexfauquette requested review from flaviendelangle and LukasTy September 29, 2023 08:42

Apply suggestions from code review

75dacd0

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 29, 2023

alexfauquette commented Oct 17, 2023

View reviewed changes

alexfauquette added 3 commits October 17, 2023 14:34

Merge remote-tracking branch 'upstream/master' into animation

b75a6ea

renovate: group react-spring

85464ba

yarn prettier

2e059cc

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 17, 2023

noraleonte reviewed Oct 17, 2023

View reviewed changes

docs/data/charts/bars/bars.md Outdated Show resolved Hide resolved

docs/data/charts/bars/bars.md Outdated Show resolved Hide resolved

docs/data/charts/bars/bars.md Outdated Show resolved Hide resolved

LukasTy reviewed Oct 18, 2023

View reviewed changes

alexfauquette and others added 5 commits October 18, 2023 14:31

origine -> origin

87f96ca

Apply suggestions from code review

4cdabcf

Co-authored-by: Nora <72460825+noraleonte@users.noreply.github.com> Co-authored-by: Lukas <llukas.tyla@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

scripts

dc52530

opt-out

6e890ed

scripts

7f86cff

LukasTy reviewed Oct 18, 2023

View reviewed changes

docs/data/charts/bars/bars.md Show resolved Hide resolved

docs/data/charts/bars/bars.md Outdated Show resolved Hide resolved

mui deleted a comment from alexfauquette Oct 18, 2023

alexfauquette added 2 commits October 18, 2023 16:49

custome reduced motion

7299e0e

fix typo

1ff3021

LukasTy approved these changes Oct 18, 2023

View reviewed changes

alexfauquette merged commit 3b48ee4 into mui:master Oct 19, 2023

oliviertassinari added the new feature New feature or request label Dec 5, 2023

oliviertassinari mentioned this pull request Dec 5, 2023

[charts] Remove animation on sparkline #11311

Merged

alexfauquette mentioned this pull request Feb 21, 2024

[charts] Allow to skip animation on sparkline bar #12160

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Support animation #9926

[charts] Support animation #9926

alexfauquette commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Aug 4, 2023

mui-bot commented Aug 4, 2023 •

edited

Loading

github-actions bot commented Aug 7, 2023

flaviendelangle Aug 8, 2023

alexfauquette Aug 8, 2023

flaviendelangle Aug 8, 2023

alexfauquette Aug 8, 2023 •

edited

Loading

romgrk Aug 11, 2023

alexfauquette Sep 27, 2023

flaviendelangle left a comment

romgrk Sep 27, 2023

alexfauquette Sep 29, 2023

romgrk Sep 29, 2023

github-actions bot commented Sep 29, 2023

alexfauquette Oct 17, 2023

noraleonte left a comment

alexfauquette commented Oct 17, 2023

LukasTy left a comment

alexfauquette commented Oct 18, 2023

LukasTy commented Oct 18, 2023

alexfauquette commented Oct 18, 2023

alexfauquette commented Oct 18, 2023

LukasTy commented Oct 18, 2023

LukasTy left a comment

[charts] Support animation #9926

[charts] Support animation #9926

Conversation

alexfauquette commented Aug 4, 2023 • edited Loading

github-actions bot commented Aug 4, 2023

mui-bot commented Aug 4, 2023 • edited Loading

github-actions bot commented Aug 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 29, 2023

Choose a reason for hiding this comment

noraleonte left a comment

Choose a reason for hiding this comment

alexfauquette commented Oct 17, 2023

LukasTy left a comment

Choose a reason for hiding this comment

alexfauquette commented Oct 18, 2023

LukasTy commented Oct 18, 2023

alexfauquette commented Oct 18, 2023

alexfauquette commented Oct 18, 2023

LukasTy commented Oct 18, 2023

Current release:

This PR

LukasTy left a comment

Choose a reason for hiding this comment

alexfauquette commented Aug 4, 2023 •

edited

Loading

mui-bot commented Aug 4, 2023 •

edited

Loading

alexfauquette Aug 8, 2023 •

edited

Loading